Skip to content

refactor: update upsert API to handle multiple rows and adjust related service methods#119

Open
Aadil-Ahmed-27 wants to merge 1 commit into
mainfrom
Upsert-API-update
Open

refactor: update upsert API to handle multiple rows and adjust related service methods#119
Aadil-Ahmed-27 wants to merge 1 commit into
mainfrom
Upsert-API-update

Conversation

@Aadil-Ahmed-27

Copy link
Copy Markdown
Contributor

…ructure

Copilot AI review requested due to automatic review settings June 1, 2026 10:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the operator CRUD layer to support new employees_internal fields and adds request validation for employee upserts.

Changes:

  • Added email_hash and password_hash to the known column list for employees_internal.
  • Modified upsert column derivation to consider keys across all provided rows and avoid updating key columns for employees_internal.
  • Added handler-level validation for employees_internal upserts to enforce presence of email_hash/mobile_no and require password_hash when email_hash is present.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/services/operator.rs Extends employee columns, changes PK mapping for employees, and adjusts upsert column/update-set generation.
src/handlers/routes.rs Adds employees_internal-specific upsert validation prior to calling the service layer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/services/operator.rs
Comment thread src/services/operator.rs Outdated
Comment on lines 1471 to 1487
// Build column list from ALL rows, excluding gtfs_id (we inject it ourselves)
// Also exclude keys whose value is JSON null — omit-null semantics so that a partial
// update (e.g. just {waybill_id, duty_date}) never triggers NOT NULL violations on
// columns the caller didn't intend to touch.
let mut cols: Vec<&str> = first_obj
.keys()
.filter(|k| k.as_str() != "gtfs_id" && !first_obj[k.as_str()].is_null())
.map(|s| s.as_str())
.collect();
let mut col_set: std::collections::HashSet<&str> = std::collections::HashSet::new();
for row in &arr {
if let Some(obj) = row.as_object() {
for (k, v) in obj.iter() {
if k.as_str() != "gtfs_id" && !v.is_null() {
col_set.insert(k.as_str());
}
}
}
}
let mut cols: Vec<&str> = col_set.into_iter().collect();
cols.sort(); // Sort for consistent SQL generation
cols.push("gtfs_id"); // gtfs_id always last

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can happen check pls @Aadil-Ahmed-27

Comment thread src/handlers/routes.rs Outdated
Comment on lines +3456 to +3494
fn validate_employee_upsert(body: &Value) -> AppResult<()> {
let rows: Vec<&Value> = if let Some(arr) = body.as_array() {
arr.iter().collect()
} else if body.is_object() {
vec![body]
} else {
return Ok(());
};

for (i, row) in rows.iter().enumerate() {
let obj = match row.as_object() {
Some(o) => o,
None => continue,
};

let has_email = obj.get("email_hash").map(|v| !v.is_null()).unwrap_or(false);
let has_mobile = obj.get("mobile_no").map(|v| !v.is_null()).unwrap_or(false);

if !has_email && !has_mobile {
return Err(AppError::BadRequest(format!(
"Row {}: at least one of 'email_hash' or 'mobile_no' must be present",
i
)));
}

if has_email {
let has_password = obj
.get("password_hash")
.map(|v| !v.is_null())
.unwrap_or(false);
if !has_password {
return Err(AppError::BadRequest(format!(
"Row {}: 'password_hash' is required when 'email_hash' is provided",
i
)));
}
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be moved to domain

Comment thread src/handlers/routes.rs Outdated
Comment on lines +3533 to +3537
// Validate employees_internal specific rules
if table == "employees_internal" {
validate_employee_upsert(&body_value)?;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to domain

Comment thread src/services/operator.rs
"employees_internal" => vec!["gtfs_id", "token_no"],
_ => vec![],
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be handled by table_pk ig

@Aadil-Ahmed-27
Aadil-Ahmed-27 force-pushed the Upsert-API-update branch 2 times, most recently from 0468d47 to 046ebe3 Compare June 3, 2026 08:46
@Aadil-Ahmed-27 Aadil-Ahmed-27 changed the title feat: add validation for employee upsert and update employee table st… refactor: update upsert API to handle multiple rows and adjust related service methods Jun 3, 2026
@Aadil-Ahmed-27
Aadil-Ahmed-27 force-pushed the Upsert-API-update branch 2 times, most recently from c6fc1cd to ac86108 Compare June 8, 2026 12:13
@pranavs6
pranavs6 requested a review from Copilot June 15, 2026 08:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Aadil-Ahmed-27
Aadil-Ahmed-27 changed the base branch from main to master-gims July 17, 2026 08:42
@Aadil-Ahmed-27
Aadil-Ahmed-27 changed the base branch from master-gims to main July 17, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants